That was fast. Lots of free time in quarantine? 😄

Additionally, don't unregister the keypress immediately. This means that auto-shifted keys can be held down.

I'd argue that the tap-then-hold behavior should also apply to shifted keys (or a flag to change whether it happens on its own). Also, TAPPING_FORCE_HOLD should disable the shifted key's autorepeat as well (and probably renamed even without that), as having autorepeat disabled for half of your typing keys seems dumber than all of them.

With this being nearly a rewrite, would you be alright with adding something similar to #8127 to merge in one go? (or is that a bad idea anyway? Kinda new to GitHub) I could code it, just want to try to get custom shifts in as nothing's happened to that for four months. Figuring out autorepeat with shifted keycodes may be tricky.

That was fast. Lots of free time in quarantine? 😄

Additionally, don't unregister the keypress immediately. This means that auto-shifted keys can be held down.

I'd argue that the tap-then-hold behavior should also apply to shifted keys (or a flag to change whether it happens on its own). Also, TAPPING_FORCE_HOLD should disable the shifted key's autorepeat as well (and probably renamed even without that), as having autorepeat disabled for half of your typing keys seems dumber than all of them.

Here's a way of thinking of the functionality I implemented: your auto-shift key is physically stuck to to a MT(MOD_LSFT, KC_NONE) key. Thinking of it this way, I believe TAPPING_FORCE_HOLD is doing the expected things (subject to IGNORE_MOD_TAP_INTERRUPT and such), i.e. it's just changing whether the MT gets held (presses shift) after tap-then-hold.

With this being nearly a rewrite, would you be alright with adding something similar to #8127 to merge in one go? (or is that a bad idea anyway? Kinda new to GitHub) I could code it, just want to try to get custom shifts in as nothing's happened to that for four months. Figuring out autorepeat with shifted keycodes may be tricky.

I will punt on it for now... It took me a few tries (and force pushes) to get an intuitive shift behaviour that I was happy with, which mostly means that the keyboard is faithfully representing its physical state with the exception of the shift key. Custom shift actions seem much more complicated to think about in the edge cases I've been testing (if I hold "1" then press and release "2", should the keyboard continue to hold the custom shifted "1" or switch it to normal "1"? What if I only tap "2", effectively requiring shift to be released - will the keyboard be pretending that "1" was released even when it wasn't?).

I think your feature is cool but probably belongs in tap dance rather than auto-shift.

I understand the comparison with the key pressing a MT and itself, but don't see that in the code or a reason to stick to it. I'm tired, might understand in the morning. Either way, I feel a way to disable keyrepeat is necessary.

I will punt on it for now... It took me a few tries (and force pushes) to get an intuitive shift behaviour that I was happy with, which mostly means that the keyboard is faithfully representing its physical state with the exception of the shift key. Custom shift actions seem much more complicated to think about in the edge cases I've been testing (if I hold "1" then press and release "2", should the keyboard continue to hold the custom shifted "1" or switch it to normal "1"? What if I only tap "2", effectively requiring shift to be released - will the keyboard be pretending that "1" was released even when it wasn't?).

See here. AutoShift keys are normally evaluated on release, are they not? You are simply making the timeout an alternate trigger to handle it. Evaluating on 'release' means we have more info about what the user is trying to do. In the case you mention, shift (but not 1) should be released and 2 tapped, then shift restored. In normal typing, this produces the exact effect as with normal keys - hold a, then press b. a will stop being repeated and there will be one b at the end. In a game, it would also likely produce the correct effect, as 1 is not a modifier (and everyone using AS has a gaming layer right now anyway, as there is no keyrepeat). Should 2 be held, it is the thing that would repeat in typing, and so the shift state should belong to it. Both of these fire when the second key is released or hits time, and so updating the shift states and current key is simple as we either swap shift, press, and restore shift without changing anything or (if hit time) swap all AS variables as if the first key did not exist.

Sorry for the wall of text. As I said above, I can code it, and if you don't want it in this pull I can always do it later, but I feel this pull and custom AutoShifts would make the feature truly complete.

I understand the comparison with the key pressing a MT and itself, but don't see that in the code or a reason to stick to it. I'm tired, might understand in the morning. Either way, I feel a way to disable keyrepeat is necessary.

I will punt on it for now... It took me a few tries (and force pushes) to get an intuitive shift behaviour that I was happy with, which mostly means that the keyboard is faithfully representing its physical state with the exception of the shift key. Custom shift actions seem much more complicated to think about in the edge cases I've been testing (if I hold "1" then press and release "2", should the keyboard continue to hold the custom shifted "1" or switch it to normal "1"? What if I only tap "2", effectively requiring shift to be released - will the keyboard be pretending that "1" was released even when it wasn't?).

See here. AutoShift keys are normally evaluated on release, are they not? You are simply making the timeout an alternate trigger to handle it. Evaluating on 'release' means we have more info about what the user is trying to do. In the case you mention, shift (but not 1) should be released and 2 tapped, then shift restored. In normal typing, this produces the exact effect as with normal keys - hold a, then press b. a will stop being repeated and there will be one b at the end. In a game, it would also likely produce the correct effect, as 1 is not a modifier (and everyone using AS has a gaming layer right now anyway, as there is no keyrepeat). Should 2 be held, it is the thing that would repeat in typing, and so the shift state should belong to it. Both of these fire when the second key is released or hits time, and so updating the shift states and current key is simple as we either swap shift, press, and restore shift without changing anything or (if hit time) swap all AS variables as if the first key did not exist.

Cool, that should be consistent with what I've implemented (except maybe the "restore shift" part - this PR won't remember how long you held "1", and you can potentially be still holding it down without it ever being auto-shifted, e.g. by rolling from "1" to "1"+"2" quickly, which I find is common for sequences like "st" on Dvorak).

The main philosophical issue I have with the custom auto shifts (should we call these "custom hold keys" or something instead, since they're not actually sending shifts?) is that if you press a bunch of them together, you have to know when you pressed each one to figure out what the keyboard might actually be sending (if your fingers are on "1" and "2", but autoshift_custom_shifts(KC_1) == KC_7, what's QMK reporting as being held down?). In the simpler pure auto-shift case, you only need to look at where your fingers are and when you pressed the last key. Pragmatically (for typing and such), this probably doesn't matter, but it seemed like a nice theoretical goal when I was staring at a keyboard viewer when testing.

I'll need to have more of a think about whether that's compatible with preventing repeats. The intuitive behaviour for me is that if you hold a key down QMK should faithfully report that key as being held down to the OS.

Sorry for the wall of text. As I said above, I can code it, and if you don't want it in this pull I can always do it later, but I feel this pull and custom AutoShifts would make the feature truly complete.

Please do go ahead and code it - it doesn't really matter what I think, plenty of QMK features don't follow my "stay true to the physical keyboard" philosophy! Whether I merge it into this PR will depend on what the QMK folks say about both - in the worst case we can have dependent PRs and rebase.

I also suspect that this PR needs to be cleaned up a little bit, particularly to support custom hold actions (there are a bunch of assumptions about what's going on with the shift key which will break when the hold actions aren't actually sending shifts). Regardless of the custom hold actions, cleaning this PR up is a good thing, so if you organize your commits in a way that makes it easy to merge cleanups/refactors that don't actually change the behaviour, I'd be interested in merging those. You can send PRs to my branch (though note I'm using a rebase workflow).

if you press a bunch of them together, you have to know when you pressed each one to figure out what the keyboard might actually be sending (if your fingers are on "1" and "2", but autoshift_custom_shifts(KC_1) == KC_7, what's QMK reporting as being held down?). In the simpler pure auto-shift case, you only need to look at where your fingers are and when you pressed the last key. Pragmatically (for typing and such), this probably doesn't matter, but it seemed like a nice theoretical goal when I was staring at a keyboard viewer when testing.

You seem to view AutoShift as a way to automate the shift layer, and elsewhere also describe issues with it being "half-active". I want it to remove the shift layer, and simply be a fast way to make every key have a different hold action (otherwise you have to use an advanced tapdance function [and, even if there was a better way, either lots of defines or XX(KC_X, LSFT(KC_X)) for every key]). It's just that the obvious second value for typing keys are their capital versions, and so the old shift layer's value for that key is the default. Describing new ones is necessary, on a small board exclamation point as period's second value makes way more sense than carat.

Also, I couldn't get the tap hold to repeat the unshifted character even with TAPPING_TERM over 1s. Don't see why it isn't working for me, the code flow looks correct. EDIT: Fixed in cleanup as many key event paths were changed but still not sure what was causing it. Only an issue if cleanup isn't merged at all I suppose.

p00ya#1
If that is merged, adding custom shifts should be a trivial task. The only things to figure out is release behavior (release both? Record what was pressed for that custom key?) and getting the keycodes to it.

For the latter there are a few options.

• Number one, which won't work, is AS(KC_A, LSFT(KC_B)). Though this would probably be best, there isn't room to store two keys with mods in a uint16 (same issue as Add the ability to send shifted keycodes with a mod-tap #2055 (comment)). Even rearranging to only pass the key's shift state wouldn't work, and that wouldn't support foreign layouts which need more to send certain keys.

• Number two is TapDance's workaround. I haven't looked into it and don't know how hard it would be to make, but this could give AutoShift more information on what's actually happening vs just what to hit.

• Number three is just a user-defined function, as in the original pull (Support for custom keys/custom shifted keys in Autoshift #8127). On the odd return values, I've realized I'm dumb and it can just return the actual thing to press, so this isn't ugly and has the same functionality as number two (one could define a macro key and no space issue).

Thoughts?

You seem to view AutoShift as a way to automate the shift layer, and elsewhere also describe issues with it being "half-active". I want it to remove the shift layer, and simply be a fast way to make every key have a different hold action (otherwise you have to use an advanced tapdance function [and, even if there was a better way, either lots of defines or XX(KC_X, LSFT(KC_X)) for every key]).

Well, yes, I think automating the shift layer is strongly implied by the name "AutoShift"! ;)

Have you considered rebranding "custom auto shift" as "custom hold actions" or similar?

p00ya#1
If that is merged, adding custom shifts should be a trivial task. The only things to figure out is release behavior (release both? Record what was pressed for that custom key?) and getting the keycodes to it.

Per my philosophy above, my personal preference would be per-key release, which probably means more state to track.

For the latter there are a few options.

• Number one, which won't work, is AS(KC_A, LSFT(KC_B)). Though this would probably be best, there isn't room to store two keys with mods in a uint16 (same issue as #2055 (comment)). Even rearranging to only pass the key's shift state wouldn't work, and that wouldn't support foreign layouts which need more to send certain keys.

You can hack around that the same way custom macros work: allocate a new keycode, and have a table that expands that to whatever you need (just a struct with 2 keycodes?).

One nice feature of this is that you can have two keys with the same keycode for the "tap" action but different keycodes for the "hold" action.

• Number two is TapDance's workaround. I haven't looked into it and don't know how hard it would be to make, but this could give AutoShift more information on what's actually happening vs just what to hit.
• Number three is just a user-defined function, as in the original pull (Support for custom keys/custom shifted keys in Autoshift #8127). On the odd return values, I've realized I'm dumb and it can just return the actual thing to press, so this isn't ugly and has the same functionality as number two (one could define a macro key and no space issue).

Thoughts?

I think a user-defined function (perhaps with a predefined one respecting AUTO_SHIFT_ALPHAS etc. for compatibility) would be clean and pragmatic.

Though I do still think the custom hold actions are something that more properly belongs in tap dance, e.g. if it the hold was part of qk_tap_dance_state_t.

Probably out of scope but worth mentioning: the really advanced option is to hook right into action_tapping.c. I considered that when rewriting auto-shift. But that code is even more fragile and less readable than what's in quantum/, which is probably why everyone is content to hook downstream in process_record.

Well, yes, I think automating the shift layer is strongly implied by the name "AutoShift"! ;)

Though I do still think the custom hold actions are something that more properly belongs in tap dance, e.g. if it the hold was part of qk_tap_dance_state_t.

I know, it felt wrong as I was writing it (how can you misinterpret 'AutoShift'?) but really, what is AS except a single-pattern from tap dance with defaults? There should be support for the most basic of configuration, especially since AS+TD is a good chunk of your available space and doing more than a few custom through TD pollutes the keymap so recreating AS isn't practical.

I think a user-defined function (perhaps with a predefined one respecting AUTO_SHIFT_ALPHAS etc. for compatibility) would be clean and pragmatic.

I'm planning on leaving the current process_auto_shift, with just a if custom at the top so that alphas could be overridden.
Actually, I just had a thought. Functions could be called on press, evaluation (with bool timeout passed or not), and release, and they press/release the key and return its keycode (which is recorded and sent to release later). I currently have some hacked-together keys that are auto-shifted, but also act as mod-taps. Things like that would be easy to make, and all three functions would be optional, so if you don't define release, what was returned by eval is released for custom keys. Due to variable-size arrays being hard/impossible, I made a 256-long bitmask of the last shiftstate for each physical key (equivalent to 8 key-value pairs if the original plan work, not bad) and that is passed to release.

I get that this is going deep into TapDance territory, but that's all AutoShift is. Heck, it probably would have been a good idea to make AutoShift just enable TD and create a bunch of advanced functions for whatever AUTO_SHIFT_XXXX are defined (isn't TD smaller firmware size even before this pull?) (EDIT: Not viable now as keyrepeat wouldn't work, but initially). I'll code this up tomorrow, if I don't hit any roadblocks I think it could be pretty clean.

This should no longer be a breaking change due to to flag changes, aside from the firmware size increase.

An option to get AS to return true for later key processing is having a constant custom keycode and editing the record to be it on any AS key down/up. Not sure where it would change anything that can be used parallel to AS, but could be useful later. That would allow other things to see that they shouldn't handle the key (how would they?) but know that the physical event happened.

This needs to be rebased upon develop and have the merge conflicts resolved.

Sorry; need another rebase/conflict fix here. There was another PR for Auto Shift that got merged.

@noroadsleft ping for merge before you force-push develop again

Please test again. I've kept the TIMER_DIFF_16 but am now performing a timer_read() from process_auto_shift.

Please test again. I've kept the TIMER_DIFF_16 but am now performing a timer_read() from process_auto_shift.

Everything's still good, same results I had before.

Thanks!